Skip to content

feat(web): acknowledged shared tx publishing - #3727

Merged
leshy merged 2 commits into
mainfrom
paul/feat/w7-publishing
Sep 6, 2026
Merged

feat(web): acknowledged shared tx publishing#3727
leshy merged 2 commits into
mainfrom
paul/feat/w7-publishing

Conversation

@paul-nechifor

@paul-nechifor paul-nechifor commented Aug 27, 2026

Copy link
Copy Markdown
Contributor
  • Adds shared input channels so multiple browser clients can send messages to a robot. Developers declare these channels in the cockpit configuration. The bridge converts incoming JSON into typed messages for modules.
  • Adds session.publish() to the web SDK. Each call receives confirmation once the bridge has published the message onto the stream.
  • Validates incoming messages and limits their size, rate, and number of pending requests.
  • Adds usage documentation and a chat input example.

@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.02348% with 56 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
dimos/e2e_tests/test_publish_browser.py 15.00% 34 Missing ⚠️
dimos/web/relay_bridge/relay_bridge_module.py 89.76% 6 Missing and 7 partials ⚠️
dimos/web/relay_bridge/module_test_support.py 97.97% 2 Missing and 1 partial ⚠️
dimos/web/cockpit.py 93.54% 1 Missing and 1 partial ⚠️
dimos/web/relay_bridge/builtin_codecs.py 66.66% 1 Missing and 1 partial ⚠️
...os/web/relay_bridge/test_relay_bridge_authoring.py 99.29% 2 Missing ⚠️
@@            Coverage Diff             @@
##             main    #3727      +/-   ##
==========================================
+ Coverage   77.83%   77.89%   +0.05%     
==========================================
  Files        1329     1333       +4     
  Lines      126868   127430     +562     
  Branches    11035    11092      +57     
==========================================
+ Hits        98746    99257     +511     
- Misses      24968    25015      +47     
- Partials     3154     3158       +4     
Flag Coverage Δ
OS-ubuntu-24.04-arm 74.62% <94.02%> (+0.07%) ⬆️
OS-ubuntu-latest 75.15% <94.02%> (+0.07%) ⬆️
Py-3.10 75.14% <94.02%> (+0.06%) ⬆️
Py-3.11 75.14% <94.02%> (+0.06%) ⬆️
Py-3.12 75.14% <94.02%> (+0.06%) ⬆️
Py-3.13 75.14% <94.02%> (+0.06%) ⬆️
Py-3.14 75.14% <94.02%> (+0.07%) ⬆️
Py-3.14t 75.14% <94.02%> (+0.06%) ⬆️
SelfHosted-Large 30.17% <22.09%> (-0.04%) ⬇️
SelfHosted-Linux 34.60% <22.09%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
dimos/web/codecs.py 100.00% <100.00%> (ø)
dimos/web/relay_bridge/_wt_session.py 92.48% <100.00%> (+0.29%) ⬆️
dimos/web/relay_bridge/conftest.py 100.00% <100.00%> (ø)
dimos/web/relay_bridge/e2e_support.py 72.13% <100.00%> (ø)
dimos/web/relay_bridge/manifest.py 98.69% <100.00%> (+0.14%) ⬆️
dimos/web/relay_bridge/protocol.py 100.00% <100.00%> (ø)
dimos/web/relay_bridge/test_protocol.py 100.00% <100.00%> (ø)
dimos/web/relay_bridge/test_relay_bridge_module.py 98.97% <100.00%> (+0.16%) ⬆️
dimos/web/relay_bridge/test_wt_client.py 98.30% <100.00%> (+0.12%) ⬆️
dimos/web/relay_bridge/test_wt_session.py 100.00% <100.00%> (ø)
... and 9 more

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@greptile-apps

greptile-apps Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This change adds acknowledged browser publishing to shared robot transmit channels, with relay correlation and SDK receipt handling. Two command-delivery reliability problems were reproduced: web/sdk/src/session.ts can accept a publish after its control stream has ended and delay failure until the local timeout, while dimos/web/relay_bridge/relay_bridge_module.py can report a publish failure after the command was already delivered. These paths should be corrected before merging.

Confidence Score: 3/5

Not safe to merge until disconnect-state handling and post-delivery acknowledgement semantics are corrected.

Focused executable checks reproduced both independent command-delivery failures with captured source and runtime output. The SDK check exercised the real session and transport lifecycle; the bridge check exercised the checked-out frame handler with both healthy and raising subscriber paths.

Files Needing Attention: web/sdk/src/session.ts needs to invalidate publish eligibility when its control stream ends. dimos/web/relay_bridge/relay_bridge_module.py needs to avoid returning a definite rejection after broadcast has occurred.

T-Rex T-Rex Logs

What T-Rex did

  • Reviewed the posted P1 finding and examined the focused control-stream publish-race test source, its runtime output, and the SDK TypeScript typecheck results.
  • Ran the authored focused reproduction and compared the healthy baseline output with the subscriber-failure output to validate the conditions of the P1 finding.
  • Validated an additional aspect of the P1 finding via a separate proof.
  • Validated contract behavior for the control-stream publish-race by running the focused reproduction against the bridge, where the healthy mode produced transport_broadcast=['command-3727'], local_subscriber_delivered=['command-3727'], and PubAck.
  • Compared two execution modes by running the authored reproduction against the _on_pub_frame method; in raising mode, command delivery still occurred, but the bridge produced PubNack with publish_failed, confirming the rejection is emitted after command delivery.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (1)

  1. General comment

    P1 publish accepts a stale connection after control-stream exit

    • Bug
      • Once #runSession() exits its control read loop, its finally block sweeps only publishes already in #pending. Until a replacement #runSession() increments #runId, the prior #manifest, #wireRunId, and #send remain valid to publish()'s connection gate. A new publish in that interval is recorded pending and sent through the prior writer. If that writer is dead, #send suppresses the write failure; no acknowledgement can arrive, so the caller receives PublishError with outcome unknown and code publish_timeout only when the 20-second local timer fires.
    • Cause
      • The connection-death cleanup at web/sdk/src/session.ts:628-633 does not invalidate the prior connection state (#send, adopted-manifest eligibility, or run identity). The publish() gate at web/sdk/src/session.ts:331-338 therefore cannot distinguish a completed control session from a live one before the replacement session starts.
    • Fix
      • Invalidate the dead run synchronously when the control loop ends, before (or together with) the pending sweep: clear #send and ensure the publish gate no longer accepts the old adopted manifest/run (for example, advance/invalidate the active run token or explicitly track control-stream liveness). This makes a publish in the reconnect interval reject locally as not_connected rather than silently timing out as unknown.

    T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "feat(web): acknowledged shared tx publis..." | Re-trigger Greptile

Comment thread web/sdk/src/session.ts
Comment thread dimos/web/relay_bridge/relay_bridge_module.py
@paul-nechifor
paul-nechifor force-pushed the paul/feat/w7-publishing branch 2 times, most recently from 89e4f57 to b78600a Compare August 28, 2026 02:30
@leshy
leshy force-pushed the paul/feat/w7-publishing branch from b78600a to 8bd5bdf Compare August 30, 2026 07:45
leshy
leshy previously approved these changes Sep 1, 2026
@paul-nechifor
paul-nechifor force-pushed the paul/feat/w7-publishing branch from 8bd5bdf to 34fbf18 Compare September 1, 2026 19:35
@paul-nechifor
paul-nechifor force-pushed the paul/feat/w7-publishing branch from 34fbf18 to 225ddce Compare September 1, 2026 20:58
@paul-nechifor
paul-nechifor force-pushed the paul/feat/w7-publishing branch from 225ddce to 69a3bb9 Compare September 1, 2026 22:00
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Sep 2, 2026
Base automatically changed from paul/feat/web-sdk-tutorial to main September 2, 2026 23:53
- Protocol v5 grows pub/pub_ack/pub_nack and error.requestId (no version
  bump: unknown messages drop, nothing misparses). New golden vectors pin
  the messages and the forwarded tx-frame shape on both sides.
- ChannelSpec gains additive publish/requiredScope manifest fields with
  mirrored domain rules (invalid_publish, invalid_scope). Normalized
  manifests now always emit both, so the golden manifests changed
  (normalization idempotence forces it).
- Channel(dir="tx", publish="shared") compiles: generated Out port, decoder
  resolved via the new resolve_decoder (registered @web_decoder or generic
  json.v1 for scalars/dicts/lists; dataclasses need an explicit decoder).
  text.json.v1 joins the built-ins. publish="exclusive" stays gated (W8).
- The relay validates pub in spec order (watch, dup id, 32 KiB cap, policy,
  scope bypass for the synthetic local principal, per-viewer + aggregate
  token buckets at maxHz, pending count/byte caps), stamps provenance, and
  forwards the JSON as a tx data frame on the robot carrier. The forwarded
  id is a relay-authored token, translated back to the viewer's own id on
  ack, so viewer-chosen ids stay untrusted and cannot collide.
- Pending settles exactly once: bridge ack/nack routed to the one live
  sender, publish_timeout via the reap sweep, robot_disconnected on robot
  death, silent release on viewer death. Counts land in /api/stats.
- The bridge dispatches carrier tx frames from the ordered control queue,
  decodes with the resolved codec (PublishContext when the signature takes
  it), verifies the declared message type, calls Out.publish(), and only
  then acks on a robot-opened one-shot @control stream. Failures nack with
  bounded messages and never recycle the session.
- SDK session.publish() -> Promise<PublishReceipt>: stable local rejection
  codes, bounded pending map, outcome "rejected" for definite failures vs
  "unknown" for connection loss/timeouts, and no automatic resend ever.
  Correlated errors bypass the hello handshake slot (they could previously
  unblock a hello() retry) and never touch the session error banner.
- examples/chat-input/ is the end-to-end publish demo, pinned by a new
  chromium e2e in CI (test_publish_browser.py) plus a Python-viewer e2e.
The block calls cockpit() without imports, so md-babel failed on it in CI.
@paul-nechifor
paul-nechifor force-pushed the paul/feat/w7-publishing branch from 817db54 to 00bb47a Compare September 2, 2026 23:53
@github-actions github-actions Bot added ready-to-merge Required CI checks have passed on this PR and removed ready-to-merge Required CI checks have passed on this PR labels Sep 2, 2026
@mustafab0

Copy link
Copy Markdown
Contributor

Adds shared input channels so multiple browser clients can send messages to a robot. Developers declare these channels in the cockpit configuration. The bridge converts incoming JSON into typed messages for modules.

Isn't this a safety risk? There should always be one clear owner of hardware such that the robot should not have conflicting instructions.

@paul-nechifor

Copy link
Copy Markdown
Contributor Author

Adds shared input channels so multiple browser clients can send messages to a robot. Developers declare these channels in the cockpit configuration. The bridge converts incoming JSON into typed messages for modules.

Isn't this a safety risk? There should always be one clear owner of hardware such that the robot should not have conflicting instructions.

This is already the case. The relay enforces one client controlling the robot at a time (it has to arm by sending teleop_start and wait to receive teleop_started).

This PR lets you declare other channels onto which different clients can send. This is not intended for teleoping, but for other reasons where two clients would need to send messages simultaneously. This is the case for the chat for example. If you have two clients viewing the same robot, they should both be able to interact with chat. You shouldn't have to "arm" chat to type something in chat.

Note that "shared" is not the default. You have to opt into it and someone should be careful to not use it to send cmd_vel commands.

@leshy
leshy added this pull request to the merge queue Sep 6, 2026
Merged via the queue into main with commit 3d65f28 Sep 6, 2026
34 checks passed
@leshy
leshy deleted the paul/feat/w7-publishing branch September 6, 2026 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants